Skip to content

Conversation

@DavidMCerdeira
Copy link
Member

@DavidMCerdeira DavidMCerdeira commented Jul 11, 2025

This commit updates mainly memory and bitmap operations to use named macros like MEM_DONT_FREE_PAGES, MEM_ALIGN_PPAGES, and BITMAP_DONT_SET instead of raw boolean values on respective functions.

@DavidMCerdeira DavidMCerdeira force-pushed the ref/avoid_bool_literals_invoke branch 2 times, most recently from ab02e52 to c0aaec7 Compare July 11, 2025 16:39
@josecm josecm self-assigned this Jul 15, 2025
@josecm
Copy link
Member

josecm commented Jul 15, 2025

@DavidMCerdeira looks good so far. Please let me know when its ready for final review

@DavidMCerdeira DavidMCerdeira force-pushed the ref/avoid_bool_literals_invoke branch from c0aaec7 to acf3c60 Compare July 15, 2025 22:42
@DavidMCerdeira DavidMCerdeira marked this pull request as ready for review July 16, 2025 07:40
@DavidMCerdeira DavidMCerdeira requested a review from danielRep July 16, 2025 07:51
@DavidMCerdeira
Copy link
Member Author

@josecm this is ready for review

@josecm josecm requested a review from miguelafsilva5 as a code owner July 25, 2025 10:27
danielRep
danielRep previously approved these changes Aug 6, 2025
Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work ! LGTM

This commit updates mainly memory and bitmap operations to use named macros
like MEM_FREE_PAGES, MEM_PPAGES_ALIGNED, and BITMAP_SET instead
of raw boolean values.

Signed-off-by: David Cerdeira <[email protected]>
Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +74 to +75
#define VGIC_GICR_ACCESS true
#define VGIC_NO_GICR_ACCESS false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt these macros have surrounding parentheses?

Suggested change
#define VGIC_GICR_ACCESS true
#define VGIC_NO_GICR_ACCESS false
#define VGIC_GICR_ACCESS (true)
#define VGIC_NO_GICR_ACCESS (false)

Comment on lines +74 to +75
#define VGIC_GICR_ACCESS true
#define VGIC_NO_GICR_ACCESS false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the phrasing:

Suggested change
#define VGIC_GICR_ACCESS true
#define VGIC_NO_GICR_ACCESS false
#define VGIC_GICR_ACCESS true
#define VGIC_NOT_GICR_ACCESS false

Comment on lines +58 to +59
#define MEM_PPAGES_ALIGNED true
#define MEM_PPAGES_NOT_ALIGNED false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define MEM_PPAGES_ALIGNED true
#define MEM_PPAGES_NOT_ALIGNED false
#define MEM_PPAGES_ALIGNED_REQUIRED true
#define MEM_PPAGES_ALIGNED_NOT_REQUIRED false

#define MEM_DONT_BROADCAST false

#define MEM_LOCKED true
#define MEM_UNLOCKED false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't love the "unlocked" phrasing. To keep consistent with the other macros maybe MEM_NOT_LOCKED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants